Feature/dream summarization - #493
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThis PR implements LLM-driven memory summarization ("Dream" consolidation) in DreamService. Configuration schema is extended with summarization thresholds and grouping strategies. DreamService now injects SummarizationService, implements end-to-end consolidation with response parsing and database sequencing, and provides helper methods for grouping and JSON handling. Comprehensive test coverage validates thresholds, LLM response parsing, failure modes, and helper utilities. Documentation is updated throughout. ChangesDreamService LLM Summarization Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/main/java/ai/labs/eddi/engine/runtime/internal/DreamService.java (1)
60-60: ⚡ Quick winInject ObjectMapper instead of creating a new instance.
Creating a new
ObjectMapperinstance bypasses Quarkus's CDI-managed Jackson configuration. Quarkus provides a pre-configured, injectableObjectMapperthat respects application-wide customization.♻️ Proposed fix
- private final ObjectMapper objectMapper = new ObjectMapper(); + private final ObjectMapper objectMapper;`@Inject` public DreamService(IUserMemoryStore userMemoryStore, SummarizationService summarizationService, - MeterRegistry meterRegistry) { + MeterRegistry meterRegistry, + ObjectMapper objectMapper) { this.userMemoryStore = userMemoryStore; this.summarizationService = summarizationService; this.meterRegistry = meterRegistry; + this.objectMapper = objectMapper; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/ai/labs/eddi/engine/runtime/internal/DreamService.java` at line 60, The DreamService currently instantiates a new ObjectMapper via the field "private final ObjectMapper objectMapper = new ObjectMapper();"; replace this with CDI injection of the Quarkus-managed ObjectMapper (e.g. remove the "new ObjectMapper()" instantiation and add an injected ObjectMapper either as a constructor parameter or a field annotated for injection) so DreamService uses the application-configured Jackson instance; update constructor and usages to reference the injected "objectMapper" field (keep the same symbol name to minimize other changes).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/java/ai/labs/eddi/configs/agents/model/AgentConfiguration.java`:
- Around line 529-560: The new summarization fields (summarizeMinEntries,
summarizeTargetEntries, summarizeGroupBy, preserveAgentProvenance,
maxSummarizationCalls, summarizationPrompt) should be extracted into an
immutable nested record to follow the project's config guideline; create a
record SummarizationConfig(int minEntries,int targetEntries,String
groupBy,boolean preserveAgentProvenance,int maxCalls,String prompt) with a
no-arg canonical default constructor that sets the current defaults
(5,2,"category",false,10 and the existing prompt), replace the six mutable
fields in DreamConfig with a single private SummarizationConfig summarization =
new SummarizationConfig(), add a public getter (and a setter on DreamConfig that
replaces the whole SummarizationConfig if project pattern requires mutability at
the container level), remove the original six fields, and update any code that
referenced summarizeMinEntries, summarizeTargetEntries, summarizeGroupBy,
preserveAgentProvenance, maxSummarizationCalls or summarizationPrompt to read
from summarization.minEntries(), summarization.targetEntries(), etc.
---
Nitpick comments:
In `@src/main/java/ai/labs/eddi/engine/runtime/internal/DreamService.java`:
- Line 60: The DreamService currently instantiates a new ObjectMapper via the
field "private final ObjectMapper objectMapper = new ObjectMapper();"; replace
this with CDI injection of the Quarkus-managed ObjectMapper (e.g. remove the
"new ObjectMapper()" instantiation and add an injected ObjectMapper either as a
constructor parameter or a field annotated for injection) so DreamService uses
the application-configured Jackson instance; update constructor and usages to
reference the injected "objectMapper" field (keep the same symbol name to
minimize other changes).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6e331845-406e-4e77-a213-b78d2eea3b74
📒 Files selected for processing (7)
HANDOFF.mddocs/changelog.mddocs/scheduling.mddocs/user-memory.mdsrc/main/java/ai/labs/eddi/configs/agents/model/AgentConfiguration.javasrc/main/java/ai/labs/eddi/engine/runtime/internal/DreamService.javasrc/test/java/ai/labs/eddi/engine/runtime/internal/DreamServiceTest.java
There was a problem hiding this comment.
Pull request overview
This PR adds LLM-driven summarization to the Dream memory consolidation service so user memories can be compressed into fewer entries, with new configuration, metrics, documentation, and tests.
Changes:
- Implements DreamService summarization using
SummarizationService, grouping, parsing, and replacement logic. - Adds DreamConfig summarization options and documents them.
- Expands DreamService tests and updates handoff/changelog documentation.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
src/main/java/ai/labs/eddi/engine/runtime/internal/DreamService.java |
Adds LLM summarization flow, metrics, grouping, parsing, and entry replacement logic. |
src/main/java/ai/labs/eddi/configs/agents/model/AgentConfiguration.java |
Adds Dream summarization configuration fields and accessors. |
src/test/java/ai/labs/eddi/engine/runtime/internal/DreamServiceTest.java |
Updates constructor setup and adds summarization/pruning/parser tests. |
docs/user-memory.md |
Documents summarization config, behavior, and metrics. |
docs/scheduling.md |
Updates Dream configuration example with summarization fields. |
docs/changelog.md |
Adds changelog entry for Dream summarization. |
HANDOFF.md |
Updates Dream feature summary and test counts. |
Comments suppressed due to low confidence (11)
src/main/java/ai/labs/eddi/engine/runtime/internal/DreamService.java:286
- The consolidation writes summaries via
upsert, whose key is(userId,key,sourceAgentId)for self/group memories and(userId,key)for global memories. If the LLM reuses any original key, this updates the original row and the following delete loop deletes that same row, so the newly consolidated memory can be lost.
for (var entry : consolidated) {
userMemoryStore.upsert(new UserMemoryEntry(
null, userId, entry.key(), entry.value(),
groupEntries.getFirst().category(),
src/main/java/ai/labs/eddi/engine/runtime/internal/DreamService.java:278
- When entries from multiple agents are summarized together (the default because
preserveAgentProvenanceis false), a self-visible summary is assigned only the first entry'ssourceAgentId. Since recall for self memories filters bysourceAgentId, deleting the originals makes the summary invisible to the other contributing agents.
Visibility mergedVisibility = mostRestrictiveVisibility(groupEntries);
String sourceAgent = groupEntries.getFirst().sourceAgentId();
Instant earliestCreated = groupEntries.stream()
src/main/java/ai/labs/eddi/engine/runtime/internal/DreamService.java:288
- Group-visible summaries are written with an empty
groupIdslist. Both datastore implementations only return group memories when theirgroup_idsoverlap the caller's groups, so summarizing group-scoped originals and deleting them makes the replacement unreachable.
userMemoryStore.upsert(new UserMemoryEntry(
null, userId, entry.key(), entry.value(),
groupEntries.getFirst().category(),
mergedVisibility, sourceAgent, List.of(),
"dream-consolidation", false, 0,
src/main/java/ai/labs/eddi/engine/runtime/internal/DreamService.java:272
summarizeTargetEntriesis not validated before capping. If it is configured as 0,subList(0, 0)leaves no consolidated entries to insert, but the code still deletes every original in the group and reports them as summarized.
// 6. Cap at target
if (consolidated.size() > config.getSummarizeTargetEntries()) {
consolidated = consolidated.subList(0, config.getSummarizeTargetEntries());
}
src/main/java/ai/labs/eddi/engine/runtime/internal/DreamService.java:295
- If one consolidated insert succeeds and a later insert fails, the catch skips deleting originals but does not roll back the summaries already inserted. That leaves duplicate/partial consolidated memories despite the safety guarantee that an insert failure preserves the original state.
for (var entry : consolidated) {
userMemoryStore.upsert(new UserMemoryEntry(
null, userId, entry.key(), entry.value(),
groupEntries.getFirst().category(),
mergedVisibility, sourceAgent, List.of(),
"dream-consolidation", false, 0,
earliestCreated, Instant.now()));
}
} catch (Exception e) {
LOGGER.warnf("[DREAM] Failed to insert consolidated entries for " +
"user='%s', group='%s': %s. Originals preserved.",
userId, group.getKey(), e.getMessage());
continue; // Insert failed → don't delete anything
src/main/java/ai/labs/eddi/engine/runtime/internal/DreamService.java:311
reducedis calculated from the number of originals rather than from successful deletes. If anydeleteEntrycall fails, the method still returns and incrementsdream.entries.summarizedas if those entries were removed, so callers and metrics report memory reduction that did not actually happen.
for (var original : groupEntries) {
try {
userMemoryStore.deleteEntry(original.id());
} catch (Exception e) {
LOGGER.warnf("[DREAM] Failed to delete original entry '%s': %s. " +
"Duplicate will be resolved by contradiction detector.",
original.id(), e.getMessage());
}
}
int reduced = groupEntries.size() - consolidated.size();
totalConsolidated += reduced;
entriesSummarizedCounter.increment(reduced);
src/main/java/ai/labs/eddi/engine/runtime/internal/DreamService.java:287
- When
summarizeGroupByisall, consolidated entries are still assigned the category of the first original entry. Summarizing a mixed fact/preference/context group will misclassify the replacements, which breaks category-based retrieval after the originals are deleted.
for (var entry : consolidated) {
userMemoryStore.upsert(new UserMemoryEntry(
null, userId, entry.key(), entry.value(),
groupEntries.getFirst().category(),
mergedVisibility, sourceAgent, List.of(),
src/main/java/ai/labs/eddi/engine/runtime/internal/DreamService.java:327
- Returning immediately for the
allgrouping ignorespreserveAgentProvenance=true, even though the config says that setting keeps entries from different agents separate. WithgroupBy=all, cross-agent entries are still consolidated together.
if ("all".equals(config.getSummarizeGroupBy())) {
// Single group
return Map.of("all", new ArrayList<>(entries));
}
src/main/java/ai/labs/eddi/engine/runtime/internal/DreamService.java:304
- This recovery path assumes the contradiction detector will clean up duplicates, but
detectContradictionsonly counts and logs conflicts and never deletes or resolves entries. A failed delete can therefore leave duplicate original+summary memories indefinitely.
LOGGER.warnf("[DREAM] Failed to delete original entry '%s': %s. " +
"Duplicate will be resolved by contradiction detector.",
src/main/java/ai/labs/eddi/engine/runtime/internal/DreamService.java:383
- The parser accepts entries as long as the
keyandvaluefields exist, even when the key is null/blank. This bypasses the memory guardrails used by normal writes and can create unusable empty-key memories (or cause partial insert failures after earlier summaries were written).
return entries.stream()
.filter(m -> m.containsKey("key") && m.containsKey("value"))
.map(m -> new ConsolidatedEntry(m.get("key"), m.get("value")))
src/main/java/ai/labs/eddi/engine/runtime/internal/DreamService.java:285
- Consolidated keys and values from the LLM are written directly without applying the memory guardrails (
maxKeyLength,maxValueLength, capacity, etc.) that normal memory writes enforce. A malformed or overly long LLM response can therefore store entries that the rest of the user-memory API would reject.
for (var entry : consolidated) {
userMemoryStore.upsert(new UserMemoryEntry(
null, userId, entry.key(), entry.value(),
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…forcement, docs fixes - Inject CDI-managed ObjectMapper instead of new instance (CodeRabbit) - Enforce maxCostPerRun via SummarizationResult token usage tracking - Fix user-memory.md config key: dreamConfig -> dream - Fix HANDOFF.md test count: 45+ -> 90 - Correct contradiction detector docs (detection-only, not resolution) - 40 DreamServiceTest tests, 0 failures
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/java/ai/labs/eddi/modules/llm/impl/SummarizationService.java`:
- Around line 148-149: The catch block in SummarizationService that currently
calls LOGGER.warnf to record summarization failures should use LOGGER.errorf
instead; locate the logging call in SummarizationService (the LOGGER.warnf(e,
"[SUMMARIZATION] Failed to summarize: provider=%s, model=%s, error=%s",
llmProvider, llmModel, e.getMessage()) invocation) and replace warnf with errorf
so failures are logged at ERROR level while keeping the same message,
parameters, and exception argument.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8cebc007-74b7-4b93-a94f-c7590283e08d
📒 Files selected for processing (6)
HANDOFF.mddocs/changelog.mddocs/user-memory.mdsrc/main/java/ai/labs/eddi/engine/runtime/internal/DreamService.javasrc/main/java/ai/labs/eddi/modules/llm/impl/SummarizationService.javasrc/test/java/ai/labs/eddi/engine/runtime/internal/DreamServiceTest.java
✅ Files skipped from review due to trivial changes (3)
- HANDOFF.md
- docs/changelog.md
- docs/user-memory.md
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/ai/labs/eddi/engine/runtime/internal/DreamService.java
…, exception handling Must-fix: eliminate redundant DB reload, align maxCostPerRun default (.50 everywhere), fix scheduling.md contradiction claim Should-fix: estimateCost includes input length, summarizeWithUsage re-throws (summarize wrapper swallows for compat), annotate dead contradictionResolution field, remove unused hasContent(), fix counts Nitpicks: buildEntriesJson uses ObjectMapper, stale Javadoc fixed 58 tests (40+12+6), 0 failures
…hecked exception wrapping 60 tests (40+12+8), 0 failures. JaCoCo: DreamService 92% line, SummarizationService 100% line.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/java/ai/labs/eddi/modules/llm/impl/SummarizationService.java`:
- Around line 151-155: In SummarizationService's RuntimeException catch block
(the handler that calls errorCounter.increment() and currently uses
LOGGER.warnf), change the logging call from LOGGER.warnf(...) to
LOGGER.errorf(...) so runtime failures are logged at ERROR level with the same
formatted message (include llmProvider, llmModel, and e.getMessage()), and keep
the increment and rethrow behavior intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d3649c37-7022-4ff9-beba-edff1d8e45d1
📒 Files selected for processing (8)
HANDOFF.mddocs/changelog.mddocs/scheduling.mdsrc/main/java/ai/labs/eddi/configs/agents/model/AgentConfiguration.javasrc/main/java/ai/labs/eddi/engine/runtime/internal/DreamService.javasrc/main/java/ai/labs/eddi/modules/llm/impl/SummarizationService.javasrc/test/java/ai/labs/eddi/engine/runtime/internal/DreamServiceTest.javasrc/test/java/ai/labs/eddi/modules/llm/impl/SummarizationServiceTest.java
✅ Files skipped from review due to trivial changes (2)
- docs/scheduling.md
- docs/changelog.md
🚧 Files skipped from review as they are similar to previous changes (3)
- src/main/java/ai/labs/eddi/configs/agents/model/AgentConfiguration.java
- src/main/java/ai/labs/eddi/engine/runtime/internal/DreamService.java
- src/test/java/ai/labs/eddi/engine/runtime/internal/DreamServiceTest.java
…ardrails High: multi-agent visibility upgrade (self→global), groupId preservation, summarizeTargetEntries >=1 validation (prevents data loss) Medium: partial insert rollback, accurate delete metrics, null-safe category grouping, LLM output key/value guardrails (blank+length) Low: SummarizationService warnf→errorf for exception handlers 71 tests (51+12+8), 0 failures. JaCoCo: DreamService 91.9% line
This pull request introduces LLM-driven memory summarization to the Dream background consolidation service, enabling EDDI agents to automatically compress related user memory entries using a configurable summarization process. The update adds new configuration options, documentation, metrics, and comprehensive tests to support and validate the feature.
Major new feature: LLM-driven summarization in DreamService
Configuration and API changes:
AgentConfiguration.DreamConfigfor fine-grained control over summarization:summarizeMinEntries,summarizeTargetEntries,summarizeGroupBy,preserveAgentProvenance,maxSummarizationCalls, andsummarizationPrompt, all with sensible defaults and setter/getter methods. [1] [2]Documentation and handoff updates:
docs/user-memory.md,docs/scheduling.md,HANDOFF.md, anddocs/changelog.mdto explain how LLM summarization works, its configuration, safety guarantees, and new metric (dream.entries.summarized). [1] [2] [3] [4] [5]HANDOFF.mdand changelog to reflect the new feature and increased test coverage. [1] [2]Metrics and monitoring:
dream.entries.summarizedto track the number of entries reduced via LLM consolidation. [1] [2]Testing and validation:
Summary of most important changes:
Feature: LLM-Driven Summarization
summarizeInteractions()inDreamService, enabling LLM-based memory consolidation with robust safety checks and configurable grouping/limits. [1] [2]Configuration: DreamConfig Enhancements
AgentConfiguration.DreamConfig. [1] [2]Documentation & Handoff
HANDOFF.mdand changelog with feature summary and test counts. [1] [2] [3] [4] [5]Metrics
dream.entries.summarized, for monitoring summarization activity. [1] [2]Testing
DreamServiceTest, for a total of 37, ensuring correctness and safety of the new feature. [1] [2]Summary by CodeRabbit
New Features
Documentation
Tests
Bug Fixes